Add image generation support - #102

we're not using QA shorthand anywhere in code, would be better to explicit full word naming like in other models, too keep consitent?

Hmm yeah, I don't mind, I just thought QueryAgentImage was a bit of a mouthful. What if we just had Image? It being imported from weaviate.agents.classes makes it clear where it comes from, I suppose

Yeah, I'd say QueryAgentImage or Image works to align with other model naming

I'm not a fan of pushing the timeout, I'd jsut keep existing default, because technically we wouldn't be able to ensure 100% reliability (e.g. because by default k8s pods would be killed after 30s of termination).
In ideal world you'd switch to async approaches if requests are expected to last that long.
So by keeping original timeout maybe it's better for user to get timeout if it's doing something very heavy (many images) to e.g. split that up to few requests

The new 2.5 image models are a lot faster, and theoretically a single image + other nodes does fit within a standard QA call in 30s, but if there is a longer final answer then it will likely push above 30s more often than not.

personally I'd like to increase the timeout so a user's first experience with this isn't that it doesn't work and times out. A first time user coming in to test this feature or using it without much API/coding experience may not know to adjust the time out without reading the docs, and it causes a bit of friction I'd like to avoid

by default k8s pods would be killed after 30s of termination

would you mind giving a bit more detail on this? the api call doesn't time out from my testing when it's above 30 seconds, when would a user likely experience this?

This works, but you could avoid this special case handling and also the user having to import and use Annotated by using a class method on the GeneratedImage type, something like

class GeneratedImage(BaseModel):
    model_config = ConfigDict(json_schema_extra={IMAGE_KEYWORD: True})

    image_prompt: str
    base64: SkipJsonSchema[str]  # hidden from the LLM's schema; server fills it

    @classmethod
    def with_options(cls, options: GeneratedImageOptions) -> type[Self]:
        class GeneratedImageWithOptions(cls):
            model_config = ConfigDict(json_schema_extra={IMAGE_KEYWORD: True, "X-image-shape": options.shape})

        return GeneratedImageWithOptions

so that

GeneratedImage.with_options(GeneratedImageOptions(shape="square")).model_json_schema()

works the same as using the default shape. Optional suggestion 🙂

I like this idea, it is a lot neater, though it needs a # type: ignore[valid-type,misc] to avoid mypy not liking the inheriting of cls.

Do you think it is overkill to have both options available for the user?

Also think it would be more user-friendly to have just

def with_options(cls, shape: ImageShape):
	...

i.e. add the option arguments directly rather than using GeneratedImageOptions, though would need updating in two places when new options become available, I think this is neater

though it needs a # type: ignore[valid-type,misc] to avoid mypy not liking the inheriting of cls

Yeah, mypy doesn't know what to do with the dynamic base class unfortunately. You can avoid that by using GeneratedImage/type[GeneratedImage] in place of cls/type[Self] so that the base class is static and mypy understands it, but then if the user subclasses GeneratedImage themself the with_options won't respect that, which is probably worse than a little type: ignore 🙂

Also think it would be more user-friendly to have just

Agreed, I think you're right that's nicer! If you did that, do you need to keep the GeneratedImageOptions too? Does having the Annotated form too have any advantages over just this classmethod? If not, I'd be tempted to remove that, and go with your options-as-args only I think

You're right, the only reason I want to keep GeneratedImageOptions is so I don't have to change the docs again 😭, but this is nicer UI, and TS client already has a different way of doing this. I'll make the switch

I just tested it and you get typing errors like this

even though it works at runtime. So it works but there will be friction, maybe I just stick to the Annotated version?

That's annoying 🫤

maybe I just stick to the Annotated version?

That's probably for the best, sorry for the distraction haha!

Haha no worries, I liked the suggestion if it worked